Skip to content

Snippet repository config#21

Merged
heliocastro merged 2 commits intomainfrom
fix/repo_config
Mar 6, 2026
Merged

Snippet repository config#21
heliocastro merged 2 commits intomainfrom
fix/repo_config

Conversation

@heliocastro
Copy link
Owner

No description provided.

Signed-off-by: Helio Chissini de Castro <helio.chissini.de.castro@cariad.technology>
Signed-off-by: Helio Chissini de Castro <helio.chissini.de.castro@cariad.technology>
@heliocastro heliocastro self-assigned this Mar 6, 2026
Copilot AI review requested due to automatic review settings March 6, 2026 09:58
@heliocastro heliocastro added the bug Something isn't working label Mar 6, 2026
@heliocastro heliocastro merged commit 68b8d79 into main Mar 6, 2026
18 checks passed
@heliocastro heliocastro deleted the fix/repo_config branch March 6, 2026 09:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds repository configuration support for snippet choices (provenance + per-snippet decisions) and introduces accompanying fixtures/tests, along with project housekeeping updates (version bump and git hook config migration).

Changes:

  • Model repo-config snippet_choices as SnippetChoices entries (provenance + list of snippet choices) and add enum parsing for snippet choice reasons.
  • Add a new repo-config fixture (tests/data/repo_config/curations.yml) and a dedicated test module for validating/parsing it.
  • Migrate git hook config from .pre-commit-config.yaml to prek.toml, and bump package version to 0.6.6.

Reviewed changes

Copilot reviewed 11 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/ort/models/config/repository_configuration.py Switch snippet_choices field to the new structured SnippetChoices model list.
src/ort/models/config/snippet_choices.py Introduces SnippetChoices (provenance + choices list).
src/ort/models/config/snippet/snippet_provenance.py Renames the config model class to SnippetProvenance.
src/ort/models/config/snippet/snippet_choice.py Adds enum conversion for reason values (YAML string → enum).
tests/data/repo_config/curations.yml Adds a repo-config example including snippet_choices and curations/excludes.
tests/test_repo_config_curations.py Adds tests to validate parsing/structure of curations.yml.
src/ort/models/provenance.py Adds a provenance model (currently includes debugging statements).
tests/data/scanoss_snippets.yml Adds a large SCANOSS/ORT result fixture (currently appears unused and contains environment-specific/internal values).
prek.toml Adds prek-based hook configuration.
.pre-commit-config.yaml Removes legacy pre-commit configuration.
pyproject.toml Bumps version to 0.6.6 and removes [tool.pyrefly] config.
uv.lock Updates locked package version to 0.6.6.
.gitignore Updates ignore entry for requirements.txt (currently looks like a typo).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +33 to +38
ORT_CONFIG_DIR: "/Users/dhxbwm7/data/ort-config"
HOME: "/Users/dhxbwm7"
SHELL: "/bin/zsh"
TERM: "xterm-ghostty"
JAVA_HOME: "/opt/homebrew/opt/openjdk@21/libexec/openjdk.jdk/Contents/Home"
GOPATH: "/Users/dhxbwm7/.local/toolchains/golang"
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fixture appears to embed machine-specific absolute paths / environment values (e.g. HOME, ORT_CONFIG_DIR, JAVA_HOME, etc.). These make the repo less portable and can unintentionally leak local environment details. Please sanitize these fields (use placeholders) or strip them from the committed fixture if they are not required by tests.

Suggested change
ORT_CONFIG_DIR: "/Users/dhxbwm7/data/ort-config"
HOME: "/Users/dhxbwm7"
SHELL: "/bin/zsh"
TERM: "xterm-ghostty"
JAVA_HOME: "/opt/homebrew/opt/openjdk@21/libexec/openjdk.jdk/Contents/Home"
GOPATH: "/Users/dhxbwm7/.local/toolchains/golang"
ORT_CONFIG_DIR: "/path/to/ort-config"
HOME: "/home/user"
SHELL: "/bin/sh"
TERM: "xterm"
JAVA_HOME: "/path/to/java/home"
GOPATH: "/home/user/go"

Copilot uses AI. Check for mistakes.
Comment on lines +1484 to +1491
apiUrl: "https://optima.cariad.digital/api/v1/scanoss"
writeToStorage: "true"
storages:
postgres:
connection:
url: "jdbc:postgresql://127.0.0.1:5888/ort?sslmode=disable"
schema: "public"
username: "postgres"
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixture includes what looks like internal/service-specific configuration (e.g. SCANOSS apiUrl, Postgres connection URL, username). Even if these are non-production values, committing real hostnames/domains can be sensitive and also makes the fixture harder to reuse. Please replace with neutral example values (e.g. https://example.invalid/..., jdbc:postgresql://localhost:...) or remove these sections if they are not needed for the tests in this repo.

Suggested change
apiUrl: "https://optima.cariad.digital/api/v1/scanoss"
writeToStorage: "true"
storages:
postgres:
connection:
url: "jdbc:postgresql://127.0.0.1:5888/ort?sslmode=disable"
schema: "public"
username: "postgres"
apiUrl: "https://example.invalid/api/v1/scanoss"
writeToStorage: "true"
storages:
postgres:
connection:
url: "jdbc:postgresql://localhost:5432/ort?sslmode=disable"
schema: "public"
username: "example_user"

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +6
---
repository:
vcs:
type: "Git"
url: "https://github.com/heliocastro/python-ort"
revision: "15544ad032100f4f6bda18c9db6be0f489c50070"
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests/data/scanoss_snippets.yml is a very large fixture (~2400 lines) and is currently not referenced by any tests in this PR. If it's not used, it should not be committed; if it is intended for upcoming tests, consider reducing it to the minimal subset needed to validate parsing/behavior to keep the repo lean and CI faster.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +26
print(v)
breakpoint()
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_provenance() currently contains debugging statements (print() and breakpoint()). This will pause execution / spam stdout if the model is ever validated, which is not acceptable for library code. Please remove these statements before merging (use logging only if strictly necessary, and behind an opt-in flag).

Suggested change
print(v)
breakpoint()

Copilot uses AI. Check for mistakes.
Comment on lines 46 to +52
description="An optional comment describing the snippet choice.",
)

@field_validator("reason", mode="before")
@classmethod
def validate_reason(cls, value):
return convert_enum(SnippetChoiceReason, value)
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While adding enum parsing for reason, note that the ORT repository-configuration schema allows choice.purl to be null / omitted, but this model currently requires purl. That means valid configs (e.g. reason: NO_RELEVANT_FINDING) will fail validation. Consider making purl optional with a default, and optionally validate that it is present only when required by the selected reason.

Copilot uses AI. Check for mistakes.
Comment on lines +71 to +79

# @model_validator(mode="before")
# @classmethod
# def validate_snippet_choice(cls, v):
# print(v)
# breakpoint()
# if not isinstance(v, dict):
# raise ValueError("SnippetChoice must be a dictionary.")
# return v
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is commented-out debug / validation code left in the model (@model_validator with print() / breakpoint()). Even commented, it adds noise and can accidentally be re-enabled later. Please remove this block before merging.

Suggested change
# @model_validator(mode="before")
# @classmethod
# def validate_snippet_choice(cls, v):
# print(v)
# breakpoint()
# if not isinstance(v, dict):
# raise ValueError("SnippetChoice must be a dictionary.")
# return v

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants